Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Example: HTTP Server #368

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Example: HTTP Server #368

merged 1 commit into from
Nov 19, 2024

Conversation

BackSlasher
Copy link
Contributor

Runs an HTTP server that will draw images on the screen. Useful for setting up a small digital frame that can be remotely controlled.
Has a friendly Python client,

Video:
https://github.com/user-attachments/assets/dd4d095b-7b5a-4d57-b403-6f1afc7d6c81

@BackSlasher
Copy link
Contributor Author

I'll fix the formatting

@BackSlasher BackSlasher force-pushed the http-server branch 2 times, most recently from b4c64c2 to 424de70 Compare November 2, 2024 20:31
@martinberlin
Copy link
Collaborator

martinberlin commented Nov 3, 2024

Cool example. It will be really nice if you could add also a:

/drawjpg endpoint, that will receive the JPG file, decompress it and draw it on the display

but knowing that you make a Python Client is also ok. It’s just that this way could be easier for someone to send a JPG image from anywhere including a browser of any mobile device.
I’ve did 2 examples of JPG decompression that you can see in www-image one uses the ROM jpegd and the other is faster and uses Larry Bank JPEGDEC component. In the v7 there is a enough PSRAM to upload and decompress a quite fat image.

@BackSlasher
Copy link
Contributor Author

Thanks for the idea, I'll have a look. If adding JPG support will be easy, I'll do it

@BackSlasher
Copy link
Contributor Author

On a second thought, would you mind merging it now, and we can add the jpg endpoint later?
I think that having this server without jpg is better than not having it at all, and we can always improve the interface later

@vroland
Copy link
Owner

vroland commented Nov 3, 2024

Looks interesting! I'll have a closer look and try it out hopefully later this week :)

@martinberlin martinberlin requested a review from vroland November 4, 2024 10:44
@martinberlin martinberlin removed their assignment Nov 4, 2024
@martinberlin
Copy link
Collaborator

martinberlin commented Nov 4, 2024

A bit of early feedback from my side here:

  1. If you clone this, switch to http-server branch and try to build it
undefined reference to `epd_lcd_deinit'

This is because is not taking esp32s3 as default target. sdkconfig.defaults should point to ESP32S3 target that is the last PCB. My advice, just copy: sdkconfig.defaults.esp32s3 to sdkconfig.defaults so the user can directly compile this for the last v7 PCB.

  1. two configuration files:

    In main/server.h, edit WIFI_SSID and WIFI_PASSWORD to match your wifi config
    In main/main.c, edit n_epd_setup to refer to the right EPD screen

This could be ideally reduced to one settings.h header file or just one of them, where you can edit a single file and just build.
For me this would be beneficial for 1st time users of this example. You know as less complicated is better so there is more people getting it right from the 1st time. Other than that for me it works great and it's pretty fast too!

Copy link
Collaborator

@martinberlin martinberlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible check my comments and the mentions done by another reviewers of the code

@BackSlasher
Copy link
Contributor Author

thank you all for the feedback!

@BackSlasher BackSlasher force-pushed the http-server branch 2 times, most recently from 7254f88 to 14c9ccb Compare November 5, 2024 18:22
Runs an HTTP server that will draw images on the screen.
Useful for setting up a small digital frame that can be remotely controlled.
@martinberlin
Copy link
Collaborator

martinberlin commented Nov 5, 2024

hello @BackSlasher please do not force push like this because is hard to follow history. Usually you force push something when you want to go back to a certain point but not when you go forward correcting stuff.

@BackSlasher
Copy link
Contributor Author

I have no problem adapting to what's convenient to you :)
I personally find squashing (and as a result, force-pushing) to keep PRs to a single commit. From now on for PRs on this repo I will avoid this.

@martinberlin martinberlin added the Waiting for feedback If it's in this state more than 2 months without feedback then the Issue/MR will be closed label Nov 8, 2024
Copy link
Collaborator

@martinberlin martinberlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @vroland if you want to try it go ahead, I like the example, and maybe at some point will contribute adding pure JPG decoding ability

@jonasschnelli
Copy link
Contributor

I have plans to test this further as soon as my boards arrive.

@vroland
Copy link
Owner

vroland commented Nov 19, 2024

Regarding the force-pushing: I don't have a problem with this, do whatever you want in the PR; I squash before merge anyway ;)

@vroland vroland merged commit 31b8b24 into vroland:main Nov 19, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Waiting for feedback If it's in this state more than 2 months without feedback then the Issue/MR will be closed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants